Skip to content

fix(repo): scan gitignored paths - #244

Merged
napetrov merged 15 commits into
intel:mainfrom
ndgrigorian:fix/scan-gitignored-paths
Oct 30, 2026
Merged

napetrov merged 15 commits into
intel:mainfrom
ndgrigorian:fix/scan-gitignored-paths

Conversation

@ndgrigorian

@ndgrigorian ndgrigorian commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This PR adds a feature to dokimi checking files provided in AGENTS.md for presence in .gitignore and ignores those files, as they are likely temporary or generated files likely not to be present in a given working tree.

Slips in a fix of a typo referencing dpctl.tensor, changing to dpnp.tensor, as dpctl.tensor no longer exists

Changes

  • adds gitignore.py
  • adds test_repo_l0_gitignore.py
  • dpctl.tensor -> dpnp.tensor in test_agent_profiles.py

Risk

Should be backwards-compatible and low-risk, only works on edge cases (i.e., actively in-use repo).

Testing

Tested against dpctl repo

Revert

Remove: test_repo_l0_gitignore.py, gitignore.py, 20261026_150000_ndgrigorian_scan-gitignored-paths.md
revert in: core.py, test_repo_l0_cases.py, repo-evaluation-contract.md, test_agent_profiles.py (slipped in typo correction)

Checklist

  • Tests added/updated and pytest passes
  • ruff, mypy, and schema checks pass
  • Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
  • Self-reviewed per "Before opening a PR" in AGENTS.md
  • No ad-hoc run artifacts committed
  • New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

…solved (not failed)

Guidance that warns about ignored files names paths the tree never

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
Copilot AI balanced review requested due to automatic review settings October 26, 2026 22:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues
What changed in this PR

Adds .gitignore-aware handling to repo scanning so that missing paths which are intentionally ignored by the repository are treated as build outputs (unresolved) rather than renames (fail).

Changes:

  • Introduces an L0 .gitignore parser/matcher (GitIgnore) aligned with gitignore(5) semantics.
  • Updates L0 path resolution to downgrade missing-but-gitignored paths/directories/globs from fail to unresolved, including the ignoring rule in the “why”.
  • Adds tests validating matcher parity with git check-ignore and new scan outcomes for gitignored paths.
File Description
dokimi/​dokimi/​repos/​l0/​gitignore.py New .gitignore parsing/matching implementation used by L0 scanning.
dokimi/​dokimi/​repos/​l0/​core.py Integrates .gitignore checks into missing-path handling and caching.
dokimi/​tests/​test_repo_l0_gitignore.py Adds parity tests against git check-ignore plus rule attribution tests.
dokimi/​tests/​test_repo_l0_cases.py Adds cases asserting missing-but-ignored paths become unresolved and include rule provenance.
dokimi/​docs/​repo-evaluation-contract.md Documents updated classification rules for missing gitignored paths.
dokimi/​changelog.d/​20261026_150000_ndgrigorian_scan-gitignored-paths.md Changelog entry describing the behavioral change.
dokimi/​tests/​test_agent_profiles.py Updates a string used by an API-name-guard test.

Review effort: Lite

Comment thread dokimi/dokimi/repos/l0/core.py Outdated
Comment thread dokimi/dokimi/repos/l0/gitignore.py Outdated
Comment thread dokimi/dokimi/repos/l0/core.py Outdated
dpctl.tensor was removed previously, moved to dpnp

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
@ndgrigorian
ndgrigorian force-pushed the fix/scan-gitignored-paths branch from 75ee694 to 9163ea3 Compare October 26, 2026 22:57
@ndgrigorian ndgrigorian changed the title Fix/scan gitignored paths fix(repo): scan gitignored paths Oct 27, 2026

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The matcher agrees with git check-ignore on the common forms (negation, nested files, **, anchoring, directory-only rules), but a legal .gitignore line can crash the scan, and two of the new unresolved paths depend on how the path is written. One bug, two risks and one nit, all inline.

What I ran, on Linux at 9163ea3:

  • make ci-local: 26 passed, 2 failed, 7 skipped; 8,591 tests; changed-line coverage 94%. The two failures are the gitleaks steps: the proxy returned 504 on the gitleaks download. Not a finding.
  • Reproductions for the findings through scan_repo, with no network.
  • A differential run of GitIgnore.ignored_by against git check-ignore --no-index over about 100 patterns (root and nested) and 41 paths.
  • 26 mutants of gitignore.py and _missing: 20 are killed. ? crossing /, the # comment skip, the \#/\! escapes and trailing-space stripping survive the full suite.

Checked, no issues

  • Git parity: no difference from git for ! under ignored directories, a/**, **/a/b, a/**/b, a**b, []], [!a], [a-z], escaped trailing spaces, CRLF lines, and nested .gitignore files.
  • Unchanged cases: with no .gitignore the result stays fail; a path inside a submodule keeps its message; a tracked file under an ignored directory is still ok.
  • Growth baseline: core.py stays under its cap and resolve_path stays at its baseline complexity.

Comment thread dokimi/dokimi/repos/l0/gitignore.py Outdated
Comment thread dokimi/dokimi/repos/l0/core.py Outdated
Comment thread dokimi/dokimi/repos/l0/core.py Outdated
Comment thread dokimi/dokimi/repos/l0/gitignore.py Outdated
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit per nit, up to 38e1a82, which fix the nit from my review:

  • 38e1a82: dokimi/dokimi/repos/l0/gitignore.py:100: the rule now keeps line (only unescaped trailing spaces removed) instead of raw.strip(), so the reason shows the rule as git read it.

make ci-local at 38e1a82: 26 passed, 2 failed (gitleaks download 504, host network), 7 skipped.

Not approving yet. Still open: the bug and the two risks from my review.

A bracket body was copied into a Python character class, so a tracked
line such as `[z-a].h` raised re.error and ended repo scan. Brackets now
follow wildmatch (POSIX classes, escapes, `]` first, a `/` inside), and
a line wildmatch aborts on (unclosed `[`, trailing `\`) matches nothing.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Each `*` was an unbounded `[^/]*`, so a line like `*a*a*a*a*a*a*a*a*a*a*b`
against a long missing name made repo scan hang. As in CPython's fnmatch,
every star but the last now takes the fixed run after it atomically.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
Every gitignore case scanned a root AGENTS.md, so dropping the guidance
file's directory from the glob probe in _glob left all tests green while a
nested `sub/*.cxx` turned from unresolved to fail.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Oct 27, 2026

Copy link
Copy Markdown
Contributor
  • Ran: full test suite, ruff, mypy and bandit on the head merged onto main; the matcher compared with git check-ignore --no-index on the thread's bracket lines and on random lines.
  • .gitignore line *a*a*a*a*a*a*a*a*a*a*b and guidance naming pkg/ followed by 50 a -> repo scan never finishes -> expected: it finishes as on main.
  • Nested pkg/AGENTS.md naming sub/*.cxx with pkg/sub/.gitignore = *.cxx -> unresolved is correct, but dropping the guidance directory from the glob probe leaves every test green -> expected: a test pins it.
  • Pushed:
    • ad212fb: brackets are read as git's wildmatch reads them, and a line git cannot read matches nothing. [!]], [z-a], [a\-z], [[:alpha:]] and [\]]x now agree with git, and the crash is gone.
    • 793fa29: stars match without backtracking, as in fnmatch. The matched set is unchanged.
    • 3e67c56: a test row for a glob in a nested guidance file.
  • Left for the author: the x probe thread (!x.cxx, [0-9]*.png) and the pkg/dist vs dist/ thread.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 3e67c56. The bracket crash is fixed and the matcher now agrees with git check-ignore on the bracket forms from my earlier thread. Two risks from my earlier review are still open, and there is one nit inline.

What I ran, on Linux at 3e67c56:

  • make ci-local: 25 passed, 3 failed, 7 skipped; 8,596 tests; changed-line coverage passes. Two failures are the gitleaks install and the commit scan that needs it (the proxy returned 504 on the download); gitleaks run by hand over the PR's six commits found no leaks. The third is test_unbalanced_quote_in_hook_completes_quickly (1.7 s against a 1 s bound in the full run), which passes alone and failed only because other test suites ran on the machine at the same time. None of the three is a finding.
  • Reproductions through scan_repo for the open threads.
  • A differential run of GitIgnore.ignored_by against git check-ignore --no-index over about 10,800 generated patterns and 101 hand-picked cases, and 45 mutants of gitignore.py and _missing.

Still open from my earlier review:

  • dokimi/dokimi/repos/l0/core.py:309: a missing directory written without the trailing slash is still fail. Guidance `pkg/dist`, pkg/.gitignore = dist/, pkg/a.c present gives fail ("pkg/ exists but pkg/dist does not"); `pkg/dist/` gives unresolved. _missing still tries only the file reading.
  • dokimi/dokimi/repos/l0/core.py:331: one x instance stands in for the whole glob. Still fail: *.cxx with !x.cxx and `pkg/*.cxx`; [0-9]*.png with `img/*.png`; gen.h with `pkg/g*.h`; foo*.h with !foox.h and `pkg/foo*.h`. The *.cxx control without the ! line gives unresolved, so the probe name decides the verdict.

I agree with Copilot that the x probe misclassifies globs under negation rules. Copilot's point on Rule.pattern does not apply: the rule now stores the line with only unescaped trailing spaces removed.

Checked, no issues

  • Brackets: [!]], [^]], []], [z-a], [a\-z], [[:alpha:]] and the unclosed or unknown-class forms agree with git, and nothing raises; unusable lines match nothing.
  • Many stars: *a*a*a*a*a*a*a*a*a*a*b against fifty a finishes in milliseconds.
  • Unchanged cases: negation, anchoring, directory-only rules, ** and nested .gitignore files agree with git; the nested guidance file case is now pinned.
  • Repo rules: changelog fragment present, contract paragraph added, no generated artifacts.

Comment thread dokimi/tests/test_repo_l0_gitignore.py
Signed-off-by: Anton Volkov <antonwolfy@gmail.com>

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit per nit, up to 62e8eb6, which fix the nit from my review:

  • 62e8eb6: dokimi/tests/test_repo_l0_gitignore.py:41: added the five suggested lines to IGNORES and the eight suggested names to PATHS.

make ci-local at 62e8eb6: 26 passed, 2 failed (host), 7 skipped.

Not approving yet. Still open: the two earlier threads of mine above nit level; @copilot-pull-request-reviewer on the glob probe substitutes x for every wildcard, so *.cxx with !x.cxx is reported fail although the glob still names ignored files.

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 62e8eb6. The matcher now reads .gitignore lines as git does, and a bad line no longer crashes the scan. What is left is in core.py: when a missing directory or glob counts as ignored. I agree with @antonwolfy's two open threads there (pkg/dist vs dist/, and the x probe for globs). The rest is in the line comments.

What I ran, at 62e8eb6:

  • make ci-local --offline: 16 passed, 1 failed, 13 skipped (8,543 tests passed). The failure is schema_compat: repo_manifest.v1.json and task_inspect.v1.json differ on main, and this PR does not touch any schema.
  • GitIgnore.ignored_by checked on [z-a].o, [!]]x, [a-]w, [[:digit:]]x, an unclosed [\ and *a*a*a*a*a*a*a*a*a*a*b against fifty a: the answers match git, nothing raises, and the many-star line returns at once.
  • The parity test, run with XDG_CONFIG_HOME pointing at an ignore file that has *.csv.

Checked, no issues

  • Brackets and stars: ad212fb reads classes as wildmatch does, and a line git cannot use matches nothing. The atomic groups in 793fa29 need Python 3.11, and that is the package's minimum.
  • Rule text in the reason: it is the line with only unescaped trailing spaces removed, so Copilot's raw.strip() point no longer applies.
  • Repo rules: the new files have the SPDX header, the contract paragraph is there, and no run artifacts are committed.

Comment thread dokimi/tests/test_repo_l0_gitignore.py Outdated
Comment thread dokimi/dokimi/repos/l0/core.py Outdated

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more nit, found by running this branch on a real repository: the reason names the .gitignore line only when the path's parent directory exists. Nothing has changed since 62e8eb6, so my earlier threads stand as they were.

What I ran, on Linux at 62e8eb6:

  • repo scan on IntelPython/dpnp at 0cf6730 (8 AGENTS.md files, 245 claims), on main (b69b15f) and on this branch: fails drop from 5 to 1.
  • Minimal trees through scan_repo for the case inline.
  • The suggested change, with tests/test_repo_l0_cases.py and tests/test_repo_l0_gitignore.py (117 passed) and scripts/check_growth.py (clean).

Checked, no issues

  • Real tree: the four claims that moved are Sphinx outputs, doc/reference/generated/ and doc/reference/*.inc in the root file and the same two written relative to doc/ in doc/AGENTS.md. Each is now unresolved and names .gitignore:15 or :16. The one fail left is TestXxx, a placeholder. No other claim changed status.
  • Links: a Markdown link to an ignored file ([API](doc/_build/html/index.html) with doc/_build/ ignored) stays fail. That looks right to me: the link is broken where the guidance is read on GitHub, and the contract paragraph covers code-span paths only.

Not approving yet: my two threads above nit level are still open.

Comment thread dokimi/changelog.d/20261026_150000_ndgrigorian_scan-gitignored-paths.md Outdated
* Fix x probe to be more general
* Fix for missing directories when lacking trailing forward slash

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
A glob was judged ignored by one made-up file, its wildcards filled in
with `x`, so the verdict depended on that letter. `*.cxx` with
`!x.cxx`, `[0-9]*.png`, `gen.h` and `gen_[a-c].h` all gave `fail`
although git ignores real matches. The probe now fills the wildcards
two ways (`x`, `dokimi-probe`) and adds each name an ignoring rule's
last segment matches (`gen.h`, `0x.png`, `gen_a.h`). It keeps only the
names the glob matches and asks the exact matcher about each. A glob
written with a trailing slash is now checked as a directory.

A missing directory was judged by the made-up name `<dir>/file`, which
a whitelist (`*`, `!*/`, `!*.py`) always ignores, so a renamed
directory read as `unresolved`. A directory now counts as ignored when
git ignores it, or would ignore every name probed inside it: `file`
and each name a `!` line re-includes.

The reason now names the `.gitignore` line on the fallbacks too, where
the parent directory is missing (`pkg/resources/cmake/`,
`pkg/include/pkg/*.h`). It also no longer says the path is "not meant
to be tracked", only that it is likely generated.

The git oracle in the parity test sets HOME and XDG_CONFIG_HOME, so a
developer's own core.excludesFile cannot change the answer. Both
whitelist test cases now include `!.gitignore`; without it the
`.gitignore` was never tracked and the cases checked nothing.

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
@ndgrigorian

Copy link
Copy Markdown
Contributor Author

@xaleryb @antonwolfy @vlad-perevezentsev
Addressed comments

@ndgrigorian
ndgrigorian requested review from antonwolfy and a balanced review from Copilot October 27, 2026 21:06

@napetrov napetrov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes: one correctness bug.

  1. Blocking: dokimi/repos/l0/core.py:386 (inside probe in _ignored_by). A missing path re-included by an anchored ! line under a whitelist reads unresolved, not fail. Repro: pkg/.gitignore = *, !*/, !/.gitignore, !/setup.py, !/keep.txt; guidance names pkg/setup.py (absent). Result: unresolved. Expected: fail. The same happens for a missing directory with !/olddir/. git check-ignore --no-index shows git tracks pkg/setup.py. The inside check builds its probe children from the rule's last segment, so !/setup.py probes setup.py/setup.py, which the anchored rule does not match. Suggested fix: skip the inside check when the path's last matching rule is a negation, and add tests for the anchored form next to the non-anchored whitelist cases.

Nits:

  • dokimi/repos/l0/gitignore.py:186 strips all trailing whitespace. git strips only unescaped trailing spaces. A trailing tab and bar\\ both differ from git check-ignore, and the differential tests do not cover them.
  • core.py:321 passes only cands[0] to _missing, so the reason names no .gitignore line for gen/out.h when pkg/gen/out.h is the ignored candidate. Passing all candidates fixes it. This is the still-open Copilot thread.
  • tests/test_repo_l0_gitignore.py:125: the 1 s timing assert may flake on a loaded box.
  • tests/test_agent_profiles.py:445 changes dpctl.tensor.empty() to dpnp.tensor.empty() (commit 9163ea3). It is not in the PR description. Split it out or explain it. The Revert section says repo-evaluation-contact.md (should be contract) and omits this file.

Once item 1 is fixed, this is approvable.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at b2444d8. The delta fixes my open points: a directory without its trailing slash, the single x probe for globs, a glob with a trailing slash, the reason on the fallback paths, and the git oracle's HOME. Two things still block: @napetrov's wrong verdict on an anchored re-include, which I reproduce, and a probe table no test pins.

I agree with @napetrov on the anchored ! re-include under a whitelist. New evidence: with *, !*/, !/setup.py, !/keep.txt in pkg/.gitignore, pkg/setup.py and pkg/keep.txt give unresolved, while git reports the ! lines.

I agree with @napetrov's points on the trailing-whitespace strip and the unrelated dpctl to dpnp change in test_agent_profiles.py.

Copilot's point on cands[0] does not apply to the verdict: the loop at core.py:315 already tries every candidate, and the fallback returns unresolved whether or not a rule matches, so only the reason text can differ.

Copilot's point on the git executable does not apply: the pre-existing _git_repo helper in test_repo_l0_cases.py already runs git unconditionally, so the PR adds no new kind of dependency.

Copilot's point on the one-second timing assertion does not apply: the input is 45 characters, a backtracking regression misses the bound by orders of magnitude, and the test passed in the full suite run.

What I ran, on a Linux review host at b2444d8:

  • make ci-local: 28 passed, 0 failed, 7 skipped; changed-line coverage passes.
  • Reproductions for the findings through scan_repo, compared with git check-ignore --no-index -v, with no network.
  • A mutation run: 46 mutants of the delta, 18 of which pass test_repo_l0_cases.py and test_repo_l0_gitignore.py. Most change only the reason text or are equivalent; the one that changes a verdict is inline.

Checked, no issues

  • Whitelists: *, !*/, !*.c keeps a missing file, a missing directory and a glob fail, as in the new CASES rows.
  • Odd lines: a trailing backslash, an unclosed [, an empty class and an unknown [[:foo:]] class give no names and no crash, and agree with git.
  • Unanchored negations: build/* with !build/keep/, gen_* with !gen_keep and *.cxx with !/keep.cxx agree with git.
  • Repo rules: changelog fragment present, contract paragraph matches the code, SPDX header on the new files, no schema or generated artifact touched.

Comment thread dokimi/tests/test_repo_l0_gitignore.py

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at b2444d8. Not ready yet: as @napetrov found and @antonwolfy confirmed, a whitelist .gitignore with anchored re-includes still hides a real rename. I reproduced it. The smaller points are in the line comments.

What I ran, at b2444d8:

  • The @napetrov case through scan_repo: with *, !*/, !/setup.py, !/olddir/ in pkg/.gitignore, the missing pkg/setup.py and pkg/olddir/ get unresolved, with the reason "pkg/.gitignore:1 (*) ignores it". Git does not ignore these paths, so they should stay fail, and a real rename is hidden.

… git does

A missing path the last matching line re-includes (`!/setup.py`,
`!/olddir/` under `*`, `!*/`) was probed as a directory, and the files
probed inside it were all ignored, so it read `unresolved`. The inside
probe now runs only for a directory no `!` line re-includes.

.gitignore lines are read as git reads them: split at `\n` only with a
`\r` before it dropped, a byte-order mark skipped, and only unescaped
trailing spaces trimmed (a tab stays; `bs\\ ` loses its space). The git
oracle reads paths with -z so such names compare unquoted.

The reason for a path or glob tried at several bases now names the
.gitignore line at any of them, not only at the root. Tests pin `_`,
`-` and `.` as class members, and the changelog says fail counts can
change for the same repository.

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>

@vchamarthi vchamarthi left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Approve

Fixes the gitignore matcher so it scans paths that are themselves gitignored, with new tests and a changelog fragment. Head 4dc53ec, merge base 59de48a.

Ran full CI locally: 24 passed / 4 failed / 7 skipped. All four ci.yml:test / Run tests reds are unrelated to this PR: two come from the ragas/langchain_core extras not being installed, and two are timing-sensitive assertions that pass standalone. The two static reds (schema_compat vs the PR base, pip-audit) are also unrelated: schema_compat flags schema files this PR does not touch (needs rebase against drift from main), and pip-audit's filelock CVEs come from a transitive dependency this PR does not touch.

Earlier review threads

Status Count Threads
Fixed, with a test 14 #1-13, #18
Accepted as-is, not a defect 4 #14-17

All 18 are GitHub-resolved; each was checked against head 4dc53ec and the described fix (or acceptance) is confirmed in code.

CI

Check Result Cause
ci.yml:test / Run tests (2) fail pre-existing, missing ragas/langchain_core extras
ci.yml:test / Run tests (2) fail pre-existing, timing-sensitive under parallel load
ci.yml:static / schema_compat vs the PR base fail needs rebase, drift from main
pip-audit.yml:pip-audit fail pre-existing, transitive filelock CVE

@codecov-commenter

codecov-commenter commented Oct 29, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 98.05825% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.31%. Comparing base (0a63f4e) to head (c6db406).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
dokimi/dokimi/repos/l0/gitignore.py 97.64% 1 Missing and 3 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
+ Coverage   93.26%   93.31%   +0.04%     
==========================================
  Files         272      273       +1     
  Lines       35117    35321     +204     
  Branches     5964     6003      +39     
==========================================
+ Hits        32752    32959     +207     
+ Misses       1547     1545       -2     
+ Partials      818      817       -1     
Files with missing lines Coverage Δ
dokimi/dokimi/repos/l0/core.py 95.60% <100.00%> (+0.29%) ⬆️
dokimi/dokimi/repos/l0/gitignore.py 97.64% <97.64%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xaleryb

xaleryb commented Oct 29, 2026

Copy link
Copy Markdown
Contributor
  • Ran: full suite, ruff, mypy and bandit on the head merged onto main; the earlier inputs (many-star line, nested glob, pkg/dist, the glob probes, pkg/gen*/, the anchored re-include) re-run through scan_repo. All of them now give the expected status. Each of ad212fb, 793fa29 and 3e67c56 still fails its test when its code is reverted.
  • Open: pkg/.gitignore = *, !*/, !.gitignore, !*.py, with pkg/a.py present, and guidance naming pkg/olddir without the slash -> unresolved (* ignores it when it is read as a file). Expected: fail, the same as pkg/olddir/. git re-includes the directory (!*/) and tracks pkg/olddir/x.py. The same happens with !/olddir/ and pkg/olddir.
  • Left for the author: whether a reading that a ! line re-includes should win over the other reading. One way: in _ignored_by, return None when a reading is not ignored and reincludes holds for it. dist under dist/ stays unresolved. The contract sentence on paths without a slash would need that exception.

tests/test_repo_l0_cases.py conflicted where this branch's gitignored-path test and main's pyargs-module test were both appended after the submodule test; both tests are kept, this branch's first.

Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
@xaleryb

xaleryb commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Merged main after #257 (0a05364): both sides added a test at the same place in tests/test_repo_l0_cases.py; both are kept. Full suite, ruff, mypy and bandit pass on Linux.

@vlad-perevezentsev vlad-perevezentsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I checked 0a05364. 4dc53ec fixes the anchored re-include that @napetrov found: a path that a ! line re-includes stays a rename. My open points are fixed as well: the fallback reasons and the changelog note.

What I ran, at 0a05364:

  • make ci-local --offline: 17 passed, 0 failed, 13 skipped (10,166 tests passed).
  • scan_repo on small trees. With *, !*/, !/.gitignore, !/setup.py, !/olddir/ in pkg/.gitignore, a missing pkg/setup.py and pkg/olddir/ are fail, and a missing pkg/gen.h is unresolved. include/* still covers a missing pkg/include/. From pkg/AGENTS.md, resources/*.h and resources/cmake/ are now unresolved and name pkg/.gitignore:1.

Checked, no issues

  • Line trimming: _trim drops only unescaped trailing spaces. A tab stays, a\ keeps its space, and bs\\ loses it, as in git.
  • Merge of main: compared with main, the branch changes only the seven files of this PR. check_file keeps the versions.check call from #259.
  • Changelog: the fragment now says that the fail count and the L0 gate result can differ from an older scan of the same repository.

@antonwolfy antonwolfy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 0a05364. The delta fixes the anchored re-include that @napetrov found, and the trailing-whitespace, changelog and fallback-reason points. Two things are worth fixing before merge: a re-included directory whose contents stay ignored still reads fail, and three verdicts the new code decides have no test.

I agree with @xaleryb that a whitelist .gitignore (*, !*/, !.gitignore, !*.py) gives unresolved for a missing pkg/olddir without the slash and fail for pkg/olddir/. That question waits for the author.

What I ran, on a Linux review host at 0a05364:

  • make ci-local: 27 passed, 1 failed, 7 skipped; changed-line coverage passes. The failure is pip-audit, which reports advisories in the lock files; this PR does not touch them.
  • Reproductions through scan_repo, compared with git check-ignore --no-index -v on real files, with no network.
  • A differential run of GitIgnore against git check-ignore (about 12,500 generated files, plus hand-picked BOM, CR, tab and escaped-space lines) and mutants of _ignored_by, _glob and _probes.

Checked, no issues

  • Line cleaning: _trim, the BOM skip and the split at \n with one \r dropped agree with git on a\t, a\ , bs\\ , CRLF and lone CR lines; the new rows kill the mutants that drop the backslash handling, the BOM skip or the tab rule.
  • Directory rules: reincludes and the directory form of ignored_by agree with git on !build/, */!*//!*.py and /*/!/src in 15 trees.
  • Docs: each sentence of the contract paragraph and the changelog fragment matches what scan_repo does, except the slash point above.
  • Repo rules: SPDX headers, one fragment, no generated artifacts, check_growth.py clean; all 10 commits carry a Signed-off-by.

Comment thread dokimi/dokimi/repos/l0/core.py Outdated
Comment thread dokimi/tests/test_repo_l0_cases.py
@vchamarthi

vchamarthi commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ OpenCodeReview: Review complete: 0 finding(s) across 7 selected item(s).

Review details
  • Commit reviewed: 1123231 against merge base 0a63f4e
  • Coverage: 7 files in 3 groups
  • Engine: OpenCodeReview v1.13.3, us.anthropic.claude-sonnet-5-5
  • Usage: 305k tokens (194k from cache), 1m59s
  • Tool calls: 16 (7 file_read, 6 code_search, 3 file_read_diff)
  • Gitignore-aware L0 scanning implementation: 2 files
  • Gitignore scanning tests: 3 files
  • Docs and changelog for gitignored path scanning: 2 files

Comment thread dokimi/tests/test_agent_profiles.py Outdated
Comment thread dokimi/tests/test_repo_l0_gitignore.py Outdated
Comment thread dokimi/tests/test_repo_l0_gitignore.py Outdated
Comment thread dokimi/dokimi/repos/l0/core.py Outdated
- The API-name guard test now asserts each shape catches its own name
  (dotted call, bare call, keyword argument), so dropping or weakening one
  shape fails it.
- The gitignore parity test compares the matcher with a pinned set of what
  `git check-ignore` printed, so it runs without git. A second test, skipped
  without git or on Windows, checks git still prints that set. It creates
  only the .gitignore files (--no-index matches names), runs `git init`
  with --template= and the isolated env.
- The names() test is split into a class no candidate fits and a `!` rule,
  each with a readable line as a positive control, and checks
  names(..., negated=True).

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
A directory that a `!*/`-style line re-includes, with every file probed
directly inside it still ignored (`out/*` or `tmp/**` with `!*/`), read
`fail`: the probe of its contents ran only when no `!` line matched the
directory. Now only a `!` line that names the path (`!/olddir/`,
`!/setup.py`, `!*.py`) skips the probe; one whose last segment is all
stars re-includes every directory and does not. `GitIgnore.reincludes`
becomes `reincluded_by` and returns the rule.

Under a whitelist (`*`, `!*/`, `!*.py`) a missing path written without a
slash stays ignored as a file (`pkg/Makefile`, and `pkg/olddir`): the
contract now says a directory needs its trailing slash there to be
checked as a rename.

New CASES rows pin the verdicts a mutation run showed were untested: the
file reading of a path without a slash, `include/*` with one re-included
file, and a directory glob under a whitelist.

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
`_probes` added a name from every ignoring rule and kept those the glob
matched, so a last segment such as `*` or `*_*` matched an unrelated rule
(`_skbuild`): `maintenance/*` named `.gitignore:1 (_skbuild)` in its
reason, and `pkg/*_*` with `pkg/` present went from `fail` to
`unresolved`. Rule names now count only when a literal starts or ends the
segment (`g*.h`, `*.png`, `gen*`); the filled-in names are still tried.

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
Comment thread dokimi/tests/test_repo_l0_gitignore.py
…nores nothing

Covers the empty-input cases the matcher skipped: a blank or
whitespace-only line, and lines that reduce to no pattern ("/", "//",
"!", "!/"), each of which git check-ignore also ignores nothing for.
Also a tree with no .gitignore, where no file is read.

Signed-off-by: Nikita Grigorian <nikita.grigorian@intel.com>
@ndgrigorian

Copy link
Copy Markdown
Contributor Author

@napetrov @antonwolfy
I don't seem to have the right to merge, so feel free to when you have the chance

@napetrov
napetrov merged commit 1cba674 into intel:main Oct 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants